Claude finished @mtmk's task in 2m 46s —— View job

PR Review

• Gather context (CLAUDE.md, PR diff)
• Review changed files
• Post inline comments for issues found
• Write review summary

Test-only, no production source changes. Changes are correct and well-targeted.

TlsPreferTest — ConnectTimeout vs outer CTS interaction

The outer CancellationTokenSource is 10 s (cts), but ConnectTimeout is now 30 s. ConnectAsync() is called without cts.Token, so if the TLS handshake stalls the client could block for up to 30 s even after the 10 s CTS fires. In practice the CTS cancels stream.ReadAsync on the server side, which resets the TCP socket and unblocks ConnectAsync with an IOException well before 30 s. The 30 s is effectively a last-resort backstop for platforms (Windows) where socket cleanup is delayed. This is fine; the test can't hang indefinitely.

TimeoutException as TLS evidence

Adding TimeoutException to the tlsRelated check is a sound third fallback: on a stalled TLS handshake the ConnectTimeout fires, producing a TimeoutException in the exception chain. A plaintext path would complete well within the timeout, so a timeout is meaningful evidence TLS was attempted. See inline comment for a minor edge-case caveat (negligible in practice).

ConnectRetryAsync budget (30 s → 60 s) with per-connection ConnectTimeout = 30 s

Tests that now pass ConnectTimeout = TimeSpan.FromSeconds(30) and call ConnectRetryAsync() (60 s budget, 1 s inter-attempt delay) effectively get 1–2 connection attempts. This is intentional — the mock servers are on loopback and should connect on the first attempt; the generous timeout just avoids spurious failures on contended runners.

Overall

No correctness, security, or async-safety issues. The clientSentConnect.Should().BeFalse() guard remains the authoritative check against the underlying bug; the timeout widening and TimeoutException addition are reliable CI-stability improvements without weakening that guard.

covered by #1147